refactor(mcp23009e): Migrate menu_navigation example to steami_screen.#395
refactor(mcp23009e): Migrate menu_navigation example to steami_screen.#395MatteoCnda1 wants to merge 1 commit intomainfrom
Conversation
nedseb
requested review from
Copilot
and removed request for
Charly-sketch and
nedseb
There was a problem hiding this comment.
Pull request overview
Intended to migrate the menu_navigation example from direct SSD1327 drawing calls to the steami_screen widget API (per PR title/description), but the diff shown only adds an explicit RST_EXPANDER pin high write before instantiating MCP23009E.
Changes:
- Drive
RST_EXPANDERhigh (reset_expander.value(1)) prior to creating theMCP23009Einstance.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Setup MCP23009E on I2C bus | ||
| i2c = I2C(1) | ||
| reset_expander = Pin("RST_EXPANDER", Pin.OUT) | ||
| reset_expander.value(1) |
There was a problem hiding this comment.
MCP23009E.__init__() already performs a hardware reset via reset_pin (drives low/high with delays). Setting reset_expander.value(1) here is redundant and inconsistent with the other examples; consider removing it, or (if you specifically need the pin high before driver init) add a short comment explaining why and set the initial output value when initializing the pin instead of a separate call.
| reset_expander.value(1) |
| # Setup MCP23009E on I2C bus | ||
| i2c = I2C(1) | ||
| reset_expander = Pin("RST_EXPANDER", Pin.OUT) | ||
| reset_expander.value(1) | ||
| mcp = MCP23009E(i2c, address=MCP23009_I2C_ADDR, reset_pin=reset_expander) | ||
|
|
||
| # Setup SSD1327 display on SPI bus |
There was a problem hiding this comment.
The PR description says this example was migrated from direct ssd1327 calls to the steami_screen widget API, but the updated file still imports/instantiates ssd1327.WS_OLED_128X128_SPI and renders the UI via display.text() with manual layout constants (e.g., X0, TITLE_Y, etc.). If this PR is meant to close #374, the steami_screen migration changes seem to be missing from this diff.
nedseb
left a comment
nedseb
left a comment
There was a problem hiding this comment.
Matteo,
La description de la PR mentionne une migration complète de menu_navigation.py vers steami_screen (remplacement de ssd1327 par SSD1327Display + Screen, utilisation de screen.menu(), screen.title(), screen.value(), suppression des constantes manuelles X0, TITLE_Y, etc.).
Cependant, le diff réel ne contient qu'un seul changement — l'ajout de reset_expander.value(1) à la ligne 30. Le reste du fichier est identique à celui de main : il utilise toujours ssd1327 directement, display.text() avec positionnement manuel, et les constantes de layout sont toujours présentes.
Le fix reset_expander.value(1) est valide (il force la pin reset à HIGH avant l'initialisation du MCP23009E pour éviter un état indéfini au boot), mais la migration vers steami_screen qui fermerait l'issue #374 n'est pas présente dans les changements.
Est-ce que les commits de migration ont été oubliés lors du push, ou est-ce qu'ils n'ont pas encore été écrits ?
Si c'est un oubli, pousse les commits manquants sur cette branche. Si la migration n'est pas encore faite, deux options :
- Restreindre cette PR au fix
reset_expander.value(1)seul (je peux ajuster le titre et retirer leCloses #374pour toi) - Compléter la migration avant de demander la review — n'hésite pas à regarder comment
spirit_level.pyoucompass_display.pyutilisentsteami_screenpour t'inspirer
Dis-moi où tu en es et on avancera ensemble.
MatteoCnda1
force-pushed
the
refactor/mcp23009e-menu-navigation
branch
from
f2f10b7 to
64f44aa
Compare
MatteoCnda1
force-pushed
the
refactor/mcp23009e-menu-navigation
branch
from
64f44aa to
2cfd1cd
Compare
Summary
Migrate
lib/mcp23009e/examples/menu_navigation.pyfrom direct SSD1327 calls to thesteami_screenwidget API. Closes #374Changes
display.text()and hand-coded positioning withscreen.menu(),screen.title(),screen.value()andscreen.subtitle()ssd1327direct instantiation withSSD1327Display+Screenwrapper@micropython.nativedecorator onis_any_pressed()for better performance in the button polling loopis_any_pressed()as a reusable helper functionX0,TITLE_Y,ITEM_Y, etc.) — layout is now handled bysteami_screenChecklist
ruff checkpassespython -m pytest tests/ -k mock -vpasses (no mock test broken)lib/mcp23009e/examples/menu_navigation.py)<scope>: <Description.>format